-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DEP PHP Support in CMS5 #476
DEP PHP Support in CMS5 #476
Conversation
1eeeb2b
to
4068c8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some failing unit tests
4068c8d
to
840aac4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
require needs to be updated.
840aac4
to
99b454b
Compare
@@ -34,6 +34,10 @@ class SecurityAdminExtension extends Extension | |||
'reset', | |||
]; | |||
|
|||
private static $url_handlers = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We change the SecurityAdmin to be a plain ModelAdmin. A side affect of that seems to be that we can create REST endpoints directly on this controller anymore.
MFA has a "reset" endpoint on Security used to trigger a force reset of a Member MFA settings.
I change that endpoint to be users/reset/$ID
instead of reset/$ID
.
…turned type for Email::send()
} catch (Exception $e) { | ||
$this->logger->info('WARNING: Account Reset Email failed to send: ' . $e->getMessage()); | ||
return false; | ||
} | ||
|
||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Email::send()
now return void
. We'll consider the Email sending a success if no exceptions are thrown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bits Sabina did are fine. I ended up reworking some of the AJAX calls. She'll review those.
Note - I found an issue with this post merge where registered methods aren't showing in SecurityAdmin - we'll deal with this post-beta - #479 |
Parent issue